feat(webapp): Annotations flot plugin #1605

pavelpashkovsky · 2022-10-06T14:36:59Z

Brief

add annotations plugin for timelines

Changes

demo screencast:

screencast.2022-10-12.14-33-30.mp4

demo:
https://pr-1605.pyroscope.io/?query=rideshare-app-golang.alloc_objects%7B%7D

Concerns

github-actions · 2022-10-06T14:41:31Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
webapp/public/assets/app.js	445.79 KB (+0.19% 🔺)	9 s (+0.19% 🔺)	4 s (+1.9% 🔺)	12.9 s
webapp/public/assets/app.css	17.95 KB (+0.37% 🔺)	359 ms (+0.37% 🔺)	0 ms (+100% 🔺)	359 ms
webapp/public/assets/styles.css	9.52 KB (+0.17% 🔺)	191 ms (+0.17% 🔺)	0 ms (+100% 🔺)	191 ms
packages/pyroscope-flamegraph/dist/index.js	92.19 KB (0%)	1.9 s (0%)	1.1 s (-20.49% 🔽)	3 s
packages/pyroscope-flamegraph/dist/index.node.js	92.08 KB (0%)	1.9 s (0%)	590 ms (+9.78% 🔺)	2.5 s
packages/pyroscope-flamegraph/dist/index.css	7.4 KB (0%)	148 ms (0%)	0 ms (+100% 🔺)	148 ms

eh-am · 2022-10-06T15:06:18Z

/create-server

webapp/javascript/components/TimelineChart/Annotations.plugin.tsx

webapp/javascript/components/TimelineChart/TimelineChartWrapper.tsx

codecov · 2022-10-07T13:55:40Z

Codecov Report

Base: 66.59% // Head: 66.61% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (528e96d) compared to base (1332ca5).
Patch has no changes to coverable lines.

❗ Current head 528e96d differs from pull request most recent head 3df0a6b. Consider uploading reports for the commit 3df0a6b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1605      +/-   ##
==========================================
+ Coverage   66.59%   66.61%   +0.03%     
==========================================
  Files         148      146       -2     
  Lines        5030     4965      -65     
  Branches     1162     1142      -20     
==========================================
- Hits         3349     3307      -42     
+ Misses       1676     1653      -23     
  Partials        5        5

Impacted Files	Coverage Δ
...pp/javascript/components/TimelineChart/markings.ts	`95.46% <ø> (+2.60%)`	⬆️
webapp/javascript/util/formatDate.ts	`87.33% <0.00%> (-4.22%)`	⬇️
webapp/javascript/redux/reducers/continuous.ts	`28.70% <0.00%> (+1.47%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

pavelpashkovsky · 2022-10-07T13:59:47Z

/create-server

Rperry2174 · 2022-10-08T19:49:04Z

/create-server

Rperry2174 · 2022-10-09T19:02:06Z

Looks great so far. Let's have the annoatation be a little bit smaller (we can focus on UI / design of it at the end if you'd like)

pavelpashkovsky · 2022-10-10T10:31:10Z

/create-server

Rperry2174 · 2022-10-10T18:51:44Z

/create-server

eh-am

Curious to hear your thoughts on why you preferred to draw on the canvas VS rendering Icons using React.

I think shouldStartAnnotationsFunctionality is unnecessary. If it's empty we don't render annotations, if it's not empty we render them. An array either has items or does not has items. Adding a third null state is unnecessary. If we need to control whether the entire plugin is activated or not, then we can create another variable enableAnnotationsPlugin or something, although I don't think this is necessary.

I know you have your own preferences, but for consistency let's use function instead of lambda functions.

webapp/javascript/components/TimelineChart/Annotations.plugin.tsx

webapp/javascript/components/TimelineChart/Selection.plugin.ts

webapp/javascript/components/TimelineChart/TimelineChartWrapper.tsx

webapp/javascript/components/TimelineChart/extractRange.ts

webapp/javascript/pages/ContinuousSingleView.tsx

pavelpashkovsky · 2022-10-11T12:42:13Z

@eh-am annotation marks (square + icon) moved outside of Flot, now it's React Component. I found a way how to position them over the timeline correctly. it used to be a problem they lost correct positioning when user changes browser window width

eh-am · 2022-10-11T13:54:15Z

/create-server

Rperry2174 · 2022-10-12T15:10:04Z

Yeah I agree with @eh-am that the "read" modal fields should both have dark backgrounds

https://user-images.githubusercontent.com/6951209/195377738-74f88832-a8ca-4b98-b4c7-a95de64c04e2.png

Maybe eventually we'll add an edit button but that will be separate

pavelpashkovsky · 2022-10-13T07:56:26Z

@eh-am , @Rperry2174 , made this input dark

pavelpashkovsky · 2022-10-13T08:03:43Z

/create-server

eh-am · 2022-10-13T16:53:02Z

RE: Icon covering the title:

Can you increase the margin a little bit? 20px would work I think

eh-am · 2022-10-13T17:09:54Z

Also is it possible to have visual feedback when hovering over an icon? In cases like the following it's difficult to tell which one I am hovering on.

webapp/javascript/pages/continuous/contextMenu/AnnotationInfo.tsx

webapp/javascript/pages/continuous/contextMenu/useAnnotationForm.ts

webapp/javascript/pages/continuous/contextMenu/AddAnnotation.menuitem.tsx

webapp/javascript/components/TimelineChart/Annotations.plugin.tsx

webapp/javascript/components/TimelineChart/AnnotationMark/index.tsx

eh-am · 2022-10-13T17:52:56Z

webapp/javascript/components/TimelineChart/ContextMenu.plugin.tsx

+          <ContextMenu
+            click={{ ...pos }}
+            containerEl={containerEl}
+            timestamp={Math.round(pos.x / 1000)}
+          />,


That's very interesting, I thought we needed the Provider there since this is rendered outside our normal react app! But it works without it... weird

you need provider if you want to import any kind of redux logic inside of the body of context menu (for example, redux selectors, useDispatch etc.). but in fact it executes functions which are already were initialized in components connected to redux. You can just take a look at webapp/javascript/pages/ContinuousSingleView.tsx. function dispatch was initialized in ContinuousSingleView, you just execute it in ContextMenu "env"

webapp/javascript/components/TimelineChart/ContextMenu.plugin.tsx

webapp/javascript/components/TimelineChart/AnnotationMark/index.tsx

cypress/integration/webapp/annotations.ts

pavelpashkovsky · 2022-10-14T08:23:11Z

@eh-am sorry, probably I deleted comments accidentally during dev process and then forgot to revert them

pavelpashkovsky · 2022-10-14T15:00:07Z

/create-server

Rperry2174 · 2022-10-17T15:01:00Z

/create-server

pavelpashkovsky · 2022-10-25T14:11:07Z

/create-server

Rperry2174

lgtm!

eh-am added the frontend Mostly JS code label Oct 6, 2022

eh-am reviewed Oct 6, 2022

View reviewed changes

webapp/javascript/components/TimelineChart/Annotations.plugin.tsx Outdated Show resolved Hide resolved

eh-am reviewed Oct 6, 2022

View reviewed changes

webapp/javascript/components/TimelineChart/TimelineChartWrapper.tsx Outdated Show resolved Hide resolved

feat: Annotations flot plugin

757cbb3

pavelpashkovsky force-pushed the flot-annotations branch from ea774c2 to 757cbb3 Compare October 7, 2022 13:50

pavelpashkovsky added 3 commits October 10, 2022 11:45

Merge branch 'main' into flot-annotations

7d4e421

chore: adjust annotations marks appearance

2e7a811

fix: test w/o icons

880cc71

pavelpashkovsky added 3 commits October 10, 2022 13:34

fix: reverse prev commit

d6887fa

chore: updating types for annotations plugin

02b70da

refactor: annotations icons

f2088cd

eh-am requested changes Oct 10, 2022

View reviewed changes

refactor: annotation marks positioning & icons

d888f8c

pavelpashkovsky added 4 commits October 11, 2022 16:12

refactor: extractRange fn

7942e4f

refactor: remove triggering hovering events

4eac5cf

refactor: rename WRAPPER_ID

a2ae0ed

refactor: remove shouldStartAnnotationsFunctionality

bdb8876

pavelpashkovsky added 3 commits October 11, 2022 17:31

refactor: refactor plugin hooks

fc39986

refactor: use fns instead of arrow fns

110c234

Merge branch 'main' into flot-annotations

d46bfca

chore: adjust annotation data modal input

0df0419

pavelpashkovsky force-pushed the flot-annotations branch from af88b7b to 0df0419 Compare October 13, 2022 07:54

chore: tests (v2)

03ef37f

pavelpashkovsky force-pushed the flot-annotations branch from b4e654f to 03ef37f Compare October 13, 2022 08:12

eh-am reviewed Oct 13, 2022

View reviewed changes

cypress/integration/webapp/annotations.ts Outdated Show resolved Hide resolved

eh-am and others added 3 commits October 13, 2022 15:22

remove cache

cfbc3d0

chore: adjust timeline title appearance for single view

abbe15b

feat: add annotation mark visual feedback on hover

0d220b8

pavelpashkovsky added 3 commits October 14, 2022 14:16

chore: resolving issues from comments

34d31f0

Merge branch 'main' into flot-annotations

ec8f5b7

fix: test raw

5f9bde6

refactor: cy test

bdb2397

pavelpashkovsky marked this pull request as ready for review October 17, 2022 14:45

pavelpashkovsky changed the title ~~feat(webapp): Annotations flot plugin [WIP]~~ feat(webapp): Annotations flot plugin Oct 17, 2022

pavelpashkovsky added 3 commits October 25, 2022 16:03

refactor: annotation mark appearance

c02c12b

refactor: assigning styles

528e96d

fix: added extra nwline for svg icon

3df0a6b

Rperry2174 approved these changes Oct 27, 2022

View reviewed changes

Rperry2174 merged commit fe80686 into main Oct 27, 2022

Rperry2174 deleted the flot-annotations branch October 27, 2022 13:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(webapp): Annotations flot plugin #1605

feat(webapp): Annotations flot plugin #1605

pavelpashkovsky commented Oct 6, 2022 •

edited

Loading

github-actions bot commented Oct 6, 2022 •

edited

Loading

eh-am commented Oct 6, 2022

codecov bot commented Oct 7, 2022 •

edited

Loading

pavelpashkovsky commented Oct 7, 2022

Rperry2174 commented Oct 8, 2022

Rperry2174 commented Oct 9, 2022

pavelpashkovsky commented Oct 10, 2022

Rperry2174 commented Oct 10, 2022

eh-am left a comment

pavelpashkovsky commented Oct 11, 2022

eh-am commented Oct 11, 2022

Rperry2174 commented Oct 12, 2022

pavelpashkovsky commented Oct 13, 2022

pavelpashkovsky commented Oct 13, 2022

eh-am commented Oct 13, 2022

eh-am commented Oct 13, 2022

eh-am Oct 13, 2022

pavelpashkovsky Oct 14, 2022

pavelpashkovsky commented Oct 14, 2022

pavelpashkovsky commented Oct 14, 2022

Rperry2174 commented Oct 17, 2022

pavelpashkovsky commented Oct 25, 2022

Rperry2174 left a comment

feat(webapp): Annotations flot plugin #1605

feat(webapp): Annotations flot plugin #1605

Conversation

pavelpashkovsky commented Oct 6, 2022 • edited Loading

Brief

Changes

Concerns

github-actions bot commented Oct 6, 2022 • edited Loading

size-limit report 📦

eh-am commented Oct 6, 2022

codecov bot commented Oct 7, 2022 • edited Loading

Codecov Report

pavelpashkovsky commented Oct 7, 2022

Rperry2174 commented Oct 8, 2022

Rperry2174 commented Oct 9, 2022

pavelpashkovsky commented Oct 10, 2022

Rperry2174 commented Oct 10, 2022

eh-am left a comment

Choose a reason for hiding this comment

pavelpashkovsky commented Oct 11, 2022

eh-am commented Oct 11, 2022

Rperry2174 commented Oct 12, 2022

pavelpashkovsky commented Oct 13, 2022

pavelpashkovsky commented Oct 13, 2022

eh-am commented Oct 13, 2022

eh-am commented Oct 13, 2022

eh-am Oct 13, 2022

Choose a reason for hiding this comment

pavelpashkovsky Oct 14, 2022

Choose a reason for hiding this comment

pavelpashkovsky commented Oct 14, 2022

pavelpashkovsky commented Oct 14, 2022

Rperry2174 commented Oct 17, 2022

pavelpashkovsky commented Oct 25, 2022

Rperry2174 left a comment

Choose a reason for hiding this comment

pavelpashkovsky commented Oct 6, 2022 •

edited

Loading

github-actions bot commented Oct 6, 2022 •

edited

Loading

codecov bot commented Oct 7, 2022 •

edited

Loading